-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #4771: Relax overriding check for polymorphic methods #4782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
3ae4c01
to
caa452c
Compare
caa452c
to
ad8e826
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
// (Code like this is found in the collection strawman) | ||
// | ||
// 2. In the case of two method types or two polytypes with matching | ||
// parameters and implicit status, merge corresppnding parameter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: corresppnding
// 2. In the case of two method types or two polytypes with matching | ||
// parameters and implicit status, merge corresppnding parameter | ||
// and result types. | ||
(tp1, tp2) match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is bound to be very inefficient since we do not optimize tupled pattern matches yet. I suggest to use nested matches instead, similar to how it is done in infoJoin.
if ctx.typeComparer.matchingPolyParams(tp1, tp2) => | ||
tp1.derivedLambdaType( | ||
mergeParamNames(tp1, tp2), | ||
(tp1.paramInfos, tp2.paramInfos).zipped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shorter and more efficient: use zipWithConserve (in Decorators.scala)
if ctx.typeComparer.matchingPolyParams(tp1, tp2) => | ||
tp1.derivedLambdaType( | ||
mergeParamNames(tp1, tp2), | ||
(tp1.paramInfos, tp2.paramInfos).zipped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zipWithConserve again
Just like in Scala 2, we now allow an override to widen the bounds of a polymorphic type parameter (this is sound because parameters can vary contravariantly). (And just like Scala 2, we continue not allowing an overriding to widen the type of a term parameter).
ad8e826
to
b87e964
Compare
Just like in Scala 2, we now allow an override to widen the bounds of a
polymorphic type parameter (this is sound because parameters can vary
contravariantly). (And just like Scala 2, we continue not allowing an
overriding to widen the type of a term parameter).